Skip to content

Fix gesture picker misalignment by using constraint ratios#18036

Merged
Arthur-Milchior merged 1 commit intoankidroid:mainfrom
theMr17:fix/gesture-picker-overlap
Mar 9, 2025
Merged

Fix gesture picker misalignment by using constraint ratios#18036
Arthur-Milchior merged 1 commit intoankidroid:mainfrom
theMr17:fix/gesture-picker-overlap

Conversation

@theMr17
Copy link
Copy Markdown
Contributor

@theMr17 theMr17 commented Feb 28, 2025

Purpose / Description

The issue occurred due to a fixed 16dp margin around the outer circles, while the central arrow image scaled dynamically based on available space. This caused misalignment in smaller gesture picker sizes.

Solution:

  • The outer circles' margins are now dynamically adjusted using constraint ratios (0.047 and 0.953), ensuring proportional scaling.
  • This change maintains visual consistency across different screen sizes and orientations.

Fixes

Approach

Previous Behavior:

  • Outer circle margins were hardcoded to 16dp.
  • The central arrow image scaled dynamically, causing layout issues on smaller screens.

New Behavior:

  • Instead of a fixed margin, outer circles now follow a constraint ratio:
    • 0.047 (4.7%) for the top margin.
    • 0.953 (95.3%) for the bottom margin.
  • This ensures proportional spacing, adapting to different screen sizes while keeping the layout intact.

Note: A 4.7% margin from the edges of the layout preserves the 16dp margin in the original picker size.

How Has This Been Tested?

Portrait Landscape
image image

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@theMr17 theMr17 changed the title Fix overlap of gesture picker images Fix gesture picker misalignment by using constraint ratios Feb 28, 2025
Copy link
Copy Markdown
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though bias should be avoided but here it seems like it fixes the issue and all views are using all 4 constraints so it won't cause an issue

android:background="@color/transparent"
app:srcCompat="@drawable/ic_gesture_swipe" />

</merge> No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? revert it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

tools:layout_height="400dp"
tools:layout_width="400dp">


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted.

@criticalAY criticalAY added the Needs Author Reply Waiting for a reply from the original author label Mar 6, 2025
@criticalAY criticalAY self-requested a review March 6, 2025 05:52
@theMr17
Copy link
Copy Markdown
Contributor Author

theMr17 commented Mar 6, 2025

Hey @criticalAY, thanks for review! Addressed your comments, please take a look.

@criticalAY criticalAY removed the Needs Author Reply Waiting for a reply from the original author label Mar 6, 2025
Copy link
Copy Markdown
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look OK to me

Copy link
Copy Markdown
Member

@BrayanDSO BrayanDSO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@BrayanDSO BrayanDSO added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Mar 7, 2025
@ShridharGoel ShridharGoel added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. and removed Needs Second Approval Has one approval, one more approval to merge labels Mar 8, 2025
@Arthur-Milchior
Copy link
Copy Markdown
Member

For some reason I can't squash merge. I wonder whether it's because there is a merge commit in this PR.

It'd be great if instead of adding more commit, you amended the previous commit. Now you can use git rebase --interactive HEAD~6 to put all in a single commit and then force push it.
Or potentially you could also pull ankidroid/main, reset softly to ankidroid/main and recreate a single commit with the commit message that should be added to our history. This way, I could merge it.

For future PR, you could just use git commit --amend or the "amend" option of whatever UI you use to create git commits, so that you change the commit instead of adding more commits

@theMr17 theMr17 force-pushed the fix/gesture-picker-overlap branch from cdaeddd to e487267 Compare March 9, 2025 02:16
@theMr17
Copy link
Copy Markdown
Contributor Author

theMr17 commented Mar 9, 2025

Ah, seems like I messed up with the rebase.

@Arthur-Milchior Arthur-Milchior added this pull request to the merge queue Mar 9, 2025
@Arthur-Milchior
Copy link
Copy Markdown
Member

Thank you very much @theMr17

Can you please let me know, are you applying to this year GSOC? I've not seen you mentioning you want to apply, but you came back to us just when the candidate phase started.
If you're not here for GSoC, then I'd really appreciate if you could wait until the end of the candidate phase to tackle new issues. We need to keep some "simple" issues for candidate, and spend our limited review time with them.
If you're applying with us, then wonderful, you're doing great contributions those past days. (Well, your contributions are great even if you're not applying)

Merged via the queue into ankidroid:main with commit 0de2eb0 Mar 9, 2025
9 checks passed
@github-actions github-actions bot added this to the 2.21 release milestone Mar 9, 2025
@github-actions github-actions bot removed Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. labels Mar 9, 2025
@theMr17
Copy link
Copy Markdown
Contributor Author

theMr17 commented Mar 9, 2025

@Arthur-Milchior It's purely a coincidence that I resumed my contributions during the candidate phase. I'm not applying for GSoC this year, but I might be applying in the future. I completely understand the need to prioritize review time and simpler issues for candidates. I'll focus on wrapping up my currently open PRs and won't start anything new until the candidate phase concludes.

Thanks for your kind words!

@david-allison
Copy link
Copy Markdown
Member

Hi there @theMr17! This is the OpenCollective Notice for PRs merged from 2025-03-01 through 2025-03-31

If you are interested in compensation for this work, the process with details is here: https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Gesture Picker screen (arrow is overlapping with circle)

6 participants